-
-
Notifications
You must be signed in to change notification settings - Fork 402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix clim_percentile
#5495
Fix clim_percentile
#5495
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5495 +/- ##
=======================================
Coverage 88.23% 88.23%
=======================================
Files 302 302
Lines 62211 62234 +23
=======================================
+ Hits 54889 54914 +25
+ Misses 7322 7320 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Looks good to me! Does look like the original implementation was rather buggy... @maximlt Ready to merge? All tests are passing except on macos... |
Ah, I guess you want to add some docs first! |
This was previously untested and I'd like to add at least a test before merging. The thing is that I'm not quite sure where to write the test, and if the test I suggest (in the OP) is right. Sure why not documenting that properly. Where would you recommend adding that? |
I'm ok with testings this as part of the bokeh plotting tests only. Technically the change applies to all backends, so arguably there should a similar test for matplotlib and plotly but I would say that is entirely optional. The approach you've suggested above looks ok to me. Happy to see this fix merged as soon as the tests goes in! |
I'd suggest putting it in |
I will merge this as I want this fix into the next release. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
The
clim_percentile
option was added in #4712 but not tested and contained two small bugs, the first one making the option never available basically. I've testedclim_percentile
as I was trying to run the plotting guide of xarray and was wondering if we had something similar to theirrobust=True
option.I'd like to write a test for this now. I'm not sure how to obtain the ranges so here's what I came up with:
The second issue I have is, if this test is alright, where to save it?